Skip to content

Conversation

@kraenhansen
Copy link
Contributor

@kraenhansen kraenhansen commented Aug 13, 2025

Description

This is a proposal to fix the following failure on CI when running the packaged app tests.

[2025/08/12 16:55:49.330] env: node: No such file or directory
[2025/08/12 16:55:49.330] usage: mkdir [-pv] [-m mode] directory ...

It seems to me that the last of these two errors are from this line indicating that $ARTIFACTS_PATH is not set correctly. I believe the reason for that, is this line failing because the machine no longer has as system-wide node.js installed.

Merging this PR will:

  • Renames the "macos13" to "macos-11" since that seemed to be a typo.
  • Move the creation of the artifacts dir into the script and stop exporting it, since since it doesn't seem to be used elsewhere anyway.
  • Rename the generic "preinstall.sh" to "install-node.sh" (since thats the only thing it's doing) and move it above the eval of the print-compas-env.sh script to allow Node.js to be installed before its usage by print-compas-env.js.
  • Refactored print-compas-env.sh to split out the platform specific envs into set-platform-env.sh since that's needed for the install-node.sh as it is right now.

Checklist

  • New tests and/or benchmarks are included
  • Documentation is changed or added
  • If this change updates the UI, screenshots/videos are added and a design review is requested
  • I have signed the MongoDB Contributor License Agreement (https://www.mongodb.com/legal/contributor-agreement)

Motivation and Context

  • Bugfix
  • New feature
  • Dependency update
  • Misc

Open Questions

  • Perhaps we would want to use an off-the-shelf "node version manager" like nvm or fnm instead?

Dependents

Types of changes

  • Backport Needed
  • Patch (non-breaking change which fixes an issue)
  • Minor (non-breaking change which adds functionality)
  • Major (fix or feature that would cause existing functionality to change)

@kraenhansen kraenhansen self-assigned this Aug 13, 2025
Copilot AI review requested due to automatic review settings August 13, 2025 06:59
@kraenhansen kraenhansen added the no release notes Fix or feature not for release notes label Aug 13, 2025
@kraenhansen kraenhansen requested a review from a team as a code owner August 13, 2025 06:59
@kraenhansen kraenhansen added the no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) label Aug 13, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR restructures the Evergreen CI setup to fix a Node.js availability issue during packaged app tests by installing Node.js earlier in the process.

  • Split platform environment setup into a separate reusable script
  • Moved Node.js installation before environment script evaluation to ensure Node.js is available
  • Consolidated artifacts directory creation into the JavaScript environment script

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
.evergreen/set-platform-env.sh New script containing platform detection logic extracted from print-compass-env.sh
.evergreen/print-debug-info.sh New script containing debug output logic extracted from install-node.sh
.evergreen/print-compass-env.sh Refactored to source platform environment from separate script
.evergreen/print-compass-env.js Added artifacts directory creation and updated require statements
.evergreen/install-node.sh Simplified to focus on Node.js installation by removing debug output
.evergreen/functions.yml Restructured setup steps to install Node.js before environment evaluation
.evergreen/buildvariants-and-tasks.yml Updated task names from macos13 to macos-11
.evergreen/buildvariants-and-tasks.in.yml Updated template task names from macos13 to macos-11

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

const npmCacheDir = path.resolve(__dirname, '..', '.deps', '.npm-cache');

printVar('ARTIFACTS_PATH', `${newPWD}/.deps`);
const artifactsPath = path.resolve(newPWD, '.deps');
Copy link

Copilot AI Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable newPWD is used but not defined in the visible code. This will cause a ReferenceError when the script runs.

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably because:

const originalPWD = process.env.PWD;
  let newPWD = originalPWD;

which.. should be fine? I guess we can ?? '' to shut it up. Add an assertion if that makes it feel better.

@kraenhansen kraenhansen force-pushed the kh/fix-evergreen-packaged-app branch from edd2186 to 7d34d7c Compare August 13, 2025 08:47
@gribnoysup gribnoysup changed the title Rearrange evergreen prepare function to install Node.js earlier fix(ci): rearrange evergreen prepare function to install Node.js earlier COMPASS-9669 COMPASS-9683 COMPASS-9687 Aug 13, 2025
@gribnoysup gribnoysup removed the no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) label Aug 13, 2025
@github-actions github-actions bot added the fix label Aug 13, 2025
@kraenhansen kraenhansen force-pushed the kh/fix-evergreen-packaged-app branch from 6375ac8 to 31b196c Compare August 13, 2025 09:50
@kraenhansen kraenhansen force-pushed the kh/fix-evergreen-packaged-app branch from 31b196c to a597c9d Compare August 13, 2025 10:03
@lerouxb lerouxb changed the title fix(ci): rearrange evergreen prepare function to install Node.js earlier COMPASS-9669 COMPASS-9683 COMPASS-9687 fix(ci): rearrange evergreen prepare function to install Node.js earlier COMPASS-9669 COMPASS-9683 COMPASS-9687 Aug 13, 2025
<% for (const group of E2E_TEST_GROUPS) { %>
<% if (['test-packaged-app-macos-11-arm', 'test-packaged-app-macos-11-x64'].includes(buildVariant.name)) { %>
- name: test-packaged-app-macos13-<%= group.number %>
- name: test-packaged-app-macos-11-<%= group.number %>
Copy link
Contributor

@lerouxb lerouxb Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof sorry. Indeed must have been a typo. All I can think of is that maybe my brain went "13 is < 14" since mongodb latest stopping being produced for mac < 14 at 8.0.6 onwards. And 11 happens to be the only version < 14 that we're running? I don't know 🤷



.evergreen/print-compass-env.js
# We cannot rely on node from the PATH, as the script we're calling is setting up that PATH.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor

@lerouxb lerouxb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Nice splits & renames that we should have probably done years ago. I'm happy if evergreen is happy 😄

Copy link
Collaborator

@gribnoysup gribnoysup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice fix and a clean-up at the same time, thanks!

@gribnoysup
Copy link
Collaborator

(I'll merge so we can do another beta and promote it to GA later)

@gribnoysup gribnoysup merged commit 29f5998 into main Aug 13, 2025
60 of 61 checks passed
@gribnoysup gribnoysup deleted the kh/fix-evergreen-packaged-app branch August 13, 2025 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix no release notes Fix or feature not for release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants